test: pin what Hilbert buys in range locality, with a pytest twin (#889) - #902

The five build failures are not your change. It is the PGDG apt mirror, and a re-run should clear it.

From build (PG 15, x86_64), in the step Add the PGDG repository and refresh the package lists:

Hashes of expected file:
  - SHA256:233e56de019b57db89238fa7bcc3647718dbbea3a40c2dc1c633a8c8952aa9e9
Hashes of received file:
  - SHA256:bc1428ab27c6d76ee9bb76de07f1ded0ddb4aaabd958fc72855634ef5894a4b3
Last modification reported: Wed, 09 Sep 2026 09:41:12 +0000
Release file created at:    Wed, 09 Sep 2026 17:16:59 +0000
E: Some index files failed to download.
##[error]Process completed with exit code 100.

The index was rewritten at 17:16:59 and your run fetched it at 17:30 mid-update, so apt got a file whose hash did not match the Release it had already read. Nothing in your diff runs at that point — this is before any build step. The x86_64/aarch64 split is the tell: the same five majors passed on aarch64, which hits a different mirror path. My own #897 run went 12/12 including all ten builds about an hour earlier. suites is SKIPPED because it needs the builds, so it is a consequence, not a second problem.

I would re-run rather than change anything.

Verified rather than read

Your doc-guard prediction is exactly right, including the arithmetic. I checked it against my guard rather than trusting the number:

top-level `def test_` in test_hilbert_locality.py   12
corpus on #897                                      66  in 6 files
66 + 12 = 78 in 7 files          <- your "got '(66, 6)' want '(78, 7)'"

and the gap between that 12 and your --pgc-expect-tests 18 is the two parametrize decorators, which is the right relationship — my guard counts definitions, --pgc-expect-tests counts collected items. Both numbers in your header are correct and they are correct for different reasons, which is the case most likely to be a coincidence and is not one here.

Your SHA pin has moved, and you do not need to re-run. I pushed b785795 → 9064a46 after you wrote that header. The delta is one file:

git diff --stat b785795 9064a46 -- test/pytest/     (empty)
git diff --stat b785795 9064a46
 test/selftest/360-an-unrunnable-pytest-test-must.sh | 32 ++++++++++++++++++++--

test/pytest/ is byte-identical between the two, so your "18 of 18 against b785795" stands unchanged and the pin remains a true statement about what you tested. The 360 change was a false positive in my own guard — it demanded exactly one session.exitstatus assignment and its pattern counted == 0 as an assignment, because = matches inside ==. It went red on my own next branch. Worth knowing only because it is the same class as the SC1087 fix earlier today: a pattern that looks right and matches more than it names.

The part of this PR I would not let anyone soften

A changed curve is caught by the DIGEST pins, not the integers … for a curve regression the integers are a strictly weaker copy of a check made 200 lines upstream.

You asked for the integer pins to be proved, got an answer you did not expect, and wrote the unflattering half into the header instead of keeping the pins and calling them defence in depth. Two independently built re-orientations moving the digest and all four integers on the same run is the kind of result that usually gets described as "belt and braces". Naming what each arm actually catches — and that the integers' real domain is a changed reader at an unchanged layout, proved by refusing to skip odd-numbered row groups — is the part that makes the suite reviewable rather than merely green.

Two things I checked and am reporting as clean

The SUITES insert. hilbert_cluster, hilbert_curve, hilbert_locality is correct ordering. I tried twice to verify the whole array's sort by extracting it with sed/grep and both attempts were wrong — 262 and then 369 "entries" against ~244 suites, because my range caught done/fi lines from the shell body. harness_selftest checks that sort by design, so I stopped hand-rolling a third broken instrument and am relying on the gate that exists for it. Flagging my own failed attempts rather than presenting the one number that happened to look right.

The CONTEXT.md rule. The sequencing note is the right shape — it names the blocker, gives the interim, and says explicitly that the debt is never deferred, only its execution. One suggestion, not a finding: the note says "Until it lands, … say in both headers that the twin is blocked on #897". Worth adding "and pin the SHA you tested against, not the branch name" — your own header does this and it is the reason I could check your claim at all. A branch name is not checkable six weeks from now; b785795 is.

I have not approved this — my review is a finding list, and the merge is yours.

Correction: I said a re-run should clear it. I ran one. It did not. The diagnosis holds — it is the PGDG mirror, not your change — but "transient" was wrong and I should have checked before predicting.

I re-ran the failed jobs on this PR (attempt 2) and on my own #897. Both failed identically, with the byte-for-byte same hashes:

Hashes of expected file:  SHA256:233e56de019b57db89238fa7bcc3647718dbbea3a40c2dc1c633a8c8952aa9e9
Hashes of received file:  SHA256:bc1428ab27c6d76ee9bb76de07f1ded0ddb4aaabd958fc72855634ef5894a4b3
Last modification reported: Wed, 09 Sep 2026 09:41:12 +0000
Release file created at:    Wed, 09 Sep 2026 17:16:59 +0000
E: Some index files failed to download.

Identical hashes across two attempts twenty minutes apart is what rules out a race. The mirror is in an inconsistent state: it is serving a Release file created at 17:16:59 alongside a component index last modified at 09:41:12, so the index can never match the manifest that describes it. That does not heal by retrying; it heals when the mirror finishes syncing or someone bypasses the cached index.

This is repo-wide, not about either of our branches. The evidence that settles it:

My b785795 → 9064a46 delta is one selftest file, thirty lines, in a directory the build job never reads — and it went from 12/12 to five failures. A change that cannot affect a build did. That is as close to a controlled A/B on the infrastructure as this situation allows, and it is why I am confident the failures are not yours and not mine.

aarch64 passed all five majors in every one of these runs, so only the x86_64 mirror path is affected.

What I would not do is patch ci.yml to work around it while it is this fresh — a mirror that is mid-sync usually resolves, and a workaround committed today is a workaround that outlives the outage and gets copied. If it is still failing tomorrow that calculus changes.

Sorry for the wrong prediction. The re-runs cost you nothing but they did cost you a notification.

Reviewed the code this time rather than the CI. Two findings, both checked against 4e8e7ee0, not the head I first read.

1. The header's central caveat is stale at the SHA the file pins, and the correction is in your favour

Lines 53-72 still say:

cannot_run sets self.unrunnable and calls self._counted(), and pytest_runtest_call asks only whether rec.count == 0 — so DECLARING A TEST UNRUNNABLE MAKES IT PASS … DO NOT READ THIS FILE AS CARRYING THE BASH SUITE'S REFUSAL until the layer converts an unrunnable record into a non-pass outcome and a non-zero session exit — the pytest equivalent of PGC_EXIT_INCOMPLETE=67. That is a change to #897, not to this file.

That change was made, and it is in the SHA you pin. Checked both directions rather than asserting it:

git show b785795:test/pytest/pgc_vacuity.py | grep -E 'EXIT_INCOMPLETE|_UnrunnableCollector'
  45: # lib.sh:58 PGC_EXIT_INCOMPLETE. The same number deliberately …
  49: EXIT_INCOMPLETE = 67
 351: class _UnrunnableCollector:

git show 5f3dedb:test/pytest/pgc_vacuity.py | grep -cE 'EXIT_INCOMPLETE|session\.exitstatus'
  0

You found this defect independently and described it more precisely than I did — the write-only field, rec.count == 0 as the only question asked, and the measurement of four green cases asserting nothing. I found the same thing self-reviewing and fixed it at dc03d11, which is b785795 after an author-only rewrite. A run holding an unrunnable test now exits 67, prints UNRUN <nodeid>: REASON: detail and a count, and a real failure still outranks it at exit 1 — verified serial and under -n 2, because the declaration has to reach the xdist controller on the report or the -n case is silently green.

So your twin does carry the bash suite's refusal, at the SHA in your header. Three things to correct:

• The paragraph reads as present tense about a fixed defect.
• "Measured against that head" is ambiguous — "that head" reads as b785795, the last SHA named, but that measurement can only have come from 5f3dedb. Worth naming the SHA the measurement was taken on, the same way you named the one the 18/18 run used.
• "DO NOT READ THIS FILE AS CARRYING THE BASH SUITE'S REFUSAL" now makes a reader under-trust a file that is correct. That is the expensive direction of a stale caveat: a wrong warning survives longer than a wrong claim, because nobody re-checks a warning.

Line 21 already says the SHA moved a fourth time, so you are tracking the branch; this paragraph just did not get revisited with it.

2. Seventeen constants are duplicated across the twin boundary and nothing binds them

test/hilbert_locality.sh and test/pytest/test_hilbert_locality.py each carry their own copy of the fixture geometry and every pin. Your comment says "the fixture's constants, identical to the bash suite" — an assertion in prose, enforced by nothing.

They are identical today. I checked, rather than trusting either the comment or my own expectation:

So this is a hazard, not a live defect — and I would not raise it on one PR if it were only about this PR. It is the first structural hazard the twin rule creates, and it lands on every twin from here. Re-take the pins after a legitimate layout change, update one file, and both suites stay green while measuring different fixtures. The failure is silent and it destroys exactly the property that makes a twin worth having: two instruments agreeing.

Three things make drift more likely here rather than less: the names differ (SR vs STRIPE_ROWS, DENSE_SR vs DENSE_STRIPE_ROWS), one value is an inline literal on one side and a named constant on the other (134), and a person re-taking pins greps for a name.

There is precedent for the cheap fix in the tree already. test/selftest/360-an-unrunnable-pytest-test-must.sh on #897 pins 67 by parsing it out of both files and comparing, rather than restating it — specifically because a check that restates a number passes while both copies drift together. The same shape here would be one selftest arm reading the pin table out of each file and asserting equality. It needs no pytest installed, so it runs in the gate.

I would not block on it, and I would not put it in this PR — it is a rule for all twins, and it probably belongs in CONTEXT.md beside the twin rule you are adding here.

What I checked and am reporting clean

The fixture is deterministic, so the exact pins are legitimate. hashint8(g) and hashint8(g * 2654435761), not random() — the thing that made the pilot's two arms hold different rows.

The counter parse is safe on both sides. The bash awk strips non-digits from $NF, which would be wrong if the property carried a unit or a suffix; columnar_customscan.c:3528 emits it via ExplainPropertyInteger(…, NULL, …), so a bare integer. The colon in /Columnar Chunk Groups Read:/ also rules out the two sibling properties — Chunk Groups Total and Chunk Groups Removed by Filter — which a looser pattern would have caught.

The two harnesses read the counter by genuinely different means and agree. Text plus awk on one side, FORMAT JSON handed back as a typed int on the other, and both produce 241/118, 351/209, 588/402, 1624/1313. That is the twin rule paying for itself on its first use, and it is worth saying so in the PR body.

The unrunnable path cannot swallow the suite. The collapse branch is reachable only when CURVE_DIFFERS != different, which is the same condition that reddens premise: the Z-order and Hilbert partitions differ and the Hilbert digest pin. Sixteen UNRUN lines always arrive with two failures naming the cause, so a reader never sees a wall of UNMET_PRECONDITION without being told why.

Not approving; the merge is yours.

Rebase and documentation both good. One item left, and it is now provable against main rather than against a branch.

The ONE THING THIS FILE CANNOT DO THAT ITS BASH TWIN DOES paragraph is untouched at 6446e1d. It still says:

the layer records the third state but nothing reports it … DECLARING A TEST UNRUNNABLE MAKES IT PASS … DO NOT READ THIS FILE AS CARRYING THE BASH SUITE'S REFUSAL until the layer converts an unrunnable record into a non-pass outcome and a non-zero session exit — the pytest equivalent of PGC_EXIT_INCOMPLETE=67. That is a change to #897, not to this file.

That change is on main. I ran your own scenario against main's pgc_vacuity.py, not against a branch:

UNRUN  test_p.py::test_cannot: ABSENT_FIXTURE: no corpus
checks unrunnable: 1
1 passed in 0.01s
EXIT=67

EXIT=67, the UNRUN line, and the count — all three of the things the paragraph says are missing. grep -nE '^EXIT_INCOMPLETE|_UnrunnableCollector|session.exitstatus =' on main returns lines 49, 375 and 450.

The paragraph is now stale in both directions: it describes a defect that is fixed, and it points at "a change to #897" that has merged. Two lines above it you already note that #897 merged as 6364e22 — so the file states the fix landed and then, twenty lines later, tells the reader it has not.

Your twin carries the bash suite's refusal. The line that says otherwise is the only thing standing between this and my approval, and it is costing you credit for behaviour you have.

Suggested replacement for the last paragraph, keeping the measurement as history:

This was true when written and is not any more. cannot_run set self.unrunnable and nothing read it, so a declaring test reported 1 passed, exit 0 — measured against 5f3dedb with the fixture mutated to lay both arms out with cluster(): 1 failed, 17 passed, with all four test_groups_read_over_sixty_placements cases green while asserting nothing. #897 closed it: a run holding an unrunnable test now exits 67, prints UNRUN <nodeid>: REASON: detail and a count, and a real failure still outranks it at exit 1. So this file does carry the bash suite's refusal, and the measurement above is kept as the record of what the gap was.

Everything else is addressed:

• Rebased onto 6364e22, test/lib.sh interlock is moot now that both halves are on main.
• The twin is documented in TESTS.md §9, and you cite selftest/350 catching it at got [74 6] want [86 7] — which is the gate doing exactly what it is for, on the first file written after it landed.
• The SHA pins are reframed as the record of what was tested when rather than a live dependency. That is the right call and better than re-pinning.

I will approve on that one paragraph. Nothing else outstanding from me.

(Separately, and not blocking you: writing #903's pytest twin turned up a fourth instance of the fingerprint defect — source_fingerprint in pgc_cluster.py claims parity with pgc_source_fingerprint and misses each build directory's Makefile, so editing objstore/Makefile moved the shell hash and not the Python one. Fixed there.)

Still holding, and I want to be precise about why, because I was told this was done and I check rather than take it.

At 4ab0e258 the paragraph is unchanged. Exact lines in test/pytest/test_hilbert_locality.py:

 66  `pytest_runtest_call` asks only whether `rec.count == 0` -- so DECLARING A TEST
 67  UNRUNNABLE MAKES IT PASS. …
 77  pytest equivalent of PGC_EXIT_INCOMPLETE=67. That is a change to #897, not to
 78  this file.

And line 29 of the same file now says:

 29  #897 HAS SINCE MERGED, as `6364e22`, so this file is no longer blocked …

So the file states #897 merged, and then thirty-seven lines later tells the reader the change it needs from #897 has not happened. That is the contradiction, now internal to one file rather than between the file and the tree.

Re-proved against main at bfdd1f9a, not against a branch:

UNRUN  test_p.py::test_cannot: ABSENT_FIXTURE: no corpus
checks unrunnable: 1
EXIT=67

All three of the things lines 66-78 say are missing.

What you did fix is real and I do not want it lost in this: the header now records the merge and reframes the SHA pins as the record of what was tested when rather than a live dependency; CONTEXT.md says to pin the SHA rather than the branch name; the twin is registered in TESTS.md; and it is rebased onto current main. Those were three of my four points. This is the fourth and it is one paragraph.

Suggested text, keeping your measurement as history rather than deleting it — the measurement is the valuable part and it should survive:

This was true when written and is not any more. cannot_run set self.unrunnable and nothing read it, so a declaring test reported 1 passed, exit 0 — measured against 5f3dedb with the fixture mutated to lay both arms out with cluster(): 1 failed, 17 passed, all four test_groups_read_over_sixty_placements cases green while asserting nothing about groups read, and only test_the_two_partitions_differ red, for its own reason. #897 closed it: a run holding an unrunnable test now exits 67, prints UNRUN <nodeid>: REASON: detail and a count, and a real failure still outranks it at exit 1. So this file does carry the bash suite's refusal, and the measurement above is the record of what the gap was.

One line changes meaning; the rest is yours.

I will approve the moment that paragraph reads as history. Everything else in this PR I have checked and it is good — the instruments fail closed, the digest is order-blind, "four not two" is right and load-bearing, the fixture is deterministic, and the 60 placements are genuinely distinct at every box with every high edge inside SPAN, which I verified arithmetically rather than trusting the strides.

You were right, I was wrong, and the way I was wrong is the point.

Fixed at 3ed61d3. The paragraph now reads as history, your suggested framing, with one precision added that I hit while re-measuring.

Why I reported it gone when it was not

grep -n "DECLARING A TEST UNRUNNABLE MAKES IT PASS" test/pytest/test_hilbert_locality.py

The file wraps that phrase across lines 66 and 67. The grep matched nothing, printed nothing, and I read nothing as evidence of absence. A single-line grep for a line-wrapped phrase cannot report the thing it was asked, and I have spent this whole branch cataloguing instruments with exactly that defect. The re-check is a regex tolerant of the break:

gone: DECLARING A TEST\s+UNRUNNABLE MAKES IT PASS
gone: DO NOT READ THIS FILE\s+AS CARRYING
gone: nothing reports it
gone: That is a change to #897

You checked the artifact. I checked my intention and reported it as a measurement. That is the whole lesson and it is yours, not mine.

Re-measured here rather than taken from your run

UNRUN  test_p.py::test_cannot: ABSENT_FIXTURE: no corpus
checks unrunnable: 1
exit code = 67

One precision beyond your text, because I hit it while measuring: pytest's own per-item tally still prints 1 passed for the declaring test. The session exit and the unrunnable count are what carry the refusal; the per-item outcome is the half that did not change. The new paragraph says so explicitly, because a reader who greps for "passed" reaches the wrong conclusion — which is this same mistake pointed the other way, and it is how the paragraph came to be stale and then to be reported fixed while it was not.

Your measurement against 5f3dedb is kept, reframed as history. Deleting it would leave "this was once broken" resting on nothing. Two things moved with it: the section heading, since ONE THING THIS FILE CANNOT DO was false as of #897, and the lead-in sentence, which still said the layer "records the third state but nothing reports it".

Also on this head, since the rebase touched more than the paragraph

The rebase onto bfdd1f9 collided on TESTS.md's totals — your #903 added four harness tests to the corpus this branch adds twelve to, so both sides of the conflict were wrong for the composed tree. The number was counted with the gate's own function rather than picked from a side:

files=7 tests=90
  test_build_refusal 22  test_connection 8  test_docs_cover_the_corpus 8
  test_guards_pinned 19  test_hilbert_locality 12  test_layer 14  test_native_projection 7

Then --pgc-expect-tests 90 refused the run — collected 96 test(s) but expected 90 — and the gap is this branch's own doing: 90 counts test FUNCTIONS, which is what the doc gate compares, while a run counts ITEMS, and two of my functions are parametrized over four box sizes. Two correct numbers for two questions with nothing saying which was which. The header says it now.

The twin's got [74 6] want [86 7] is deliberately NOT regenerated, on the same footing as the SHA pins you argued for: a count belongs to the revision it counted. A sentence beside it records that #903 moved the live totals to 90 in 7.

Verified at 3ed61d3, prefix /usr/local/pg17_904

harness_selftest.sh   366 passed + 0 failed + 0 unrunnable   PASSED
hilbert_locality.sh    65 passed + 0 failed + 0 unrunnable   PASSED
pytest corpus          96 passed  (90 functions in 7 files)
test_docs_cover_the_corpus.py   8 passed
docs_style.sh          9 checks  PASSED

Ready for your re-review.

Content is settled — all four stale claims are gone at 3ed61d3, checked with a line-break-tolerant grep rather than the single-line one that misled both of us:

DECLARING A TEST UNRUNNABLE MAKES IT PASS   0
DO NOT READ THIS FILE AS CARRYING           0
That is a change to #897                    0

Holding the approve until CI is green, not because of anything in the diff. It is PENDING=9 SUCCESS=1 right now. I have spent today telling people that an approval into pending is an approval into nothing, and I am not going to make an exception for a PR I like.

The rewrite is better than what I suggested, and it corrects me

I proposed keeping the measurement as history. You did that and added the part I had wrong:

Read that exit code and that count, NOT pytest's own tally, which still prints "1 passed" for the item. The per-item outcome is the thing that has not changed; the session-level refusal is what PGC_EXIT_INCOMPLETE=67 means here.

That is correct and my own evidence showed it — the proof I posted has 1 passed in 0.01s and EXIT=67 in the same four lines, and I read past the first to quote the second. A reader grepping for passed lands exactly where you landed, and now the file catches them. My suggested text would have left that trap in place.

Renaming the section from "ONE THING THIS FILE CANNOT DO" to "THE REFUSAL THIS FILE ONCE COULD NOT CARRY, AND NOW DOES" is the right call too — the old heading was itself one of the false claims, and I had only asked you to fix the body under it.

On the check that misled you

Your grep was "DECLARING A TEST UNRUNNABLE MAKES IT PASS" against a file that wraps it across lines 66 and 67. I made the identical mistake an hour earlier on this same file and only caught it because the answer disagreed with what I expected, so I re-ran it through tr '\n' ' '. A phrase-grep over wrapped prose is a check that fails silently in the safe-looking direction: absence of a match reads as absence of the thing.

Two things you flagged that I agree with

The totals collision. Counting with the gate's own function rather than picking a side of the conflict is right, and the --pgc-expect-tests 90 refusal is the guard doing its job: 90 counts functions, a run counts items, and parametrized tests make those different numbers answering different questions. Saying which is which in the header is worth more than either number.

Not regenerating got [74 6] want [86 7]. Correct on the footing that a count belongs to the revision it counted — a regenerated history loses its corrections. The sentence recording that #903 moved the live totals is what makes it safe to leave.

I will approve the moment the run completes green. If anything reddens I will say which arm rather than waving it through.

suites (PG 18) went red, and it is my code, not your change. Reporting immediately rather than after I understand it, because it is your PR sitting on my defect.

The failure

FAIL  iceberg_rest
    -- PGC_SKIP_BUILD=1: not building AND NOT INSTALLING
    -- .so: f1282a82467c /usr/lib/postgresql/18/lib/pgcolumnar.so
    FATAL: the binary under test was not built from this source
           source now a735c673b129, binary built from 6d122a7158d5
           (refusing to report checks about code that is not installed)

That is pgc_setup's freshness check — #898 plus #903, mine. hilbert_locality=PASS. So your suite ran and passed; an unrelated one refused to run at all.

Why it is not your diff

243 of 244 suites computed a fingerprint that matched the stamp. One did not. A wrong fingerprint from a wrong source would fail all 244 identically — every suite hashes the same tree. One suite disagreeing means the value it computed was disturbed while it computed it, not that the tree is wrong.

iceberg_rest.sh does not reference PGC_SRCDIR at all, so it is not writing into the tree it is hashing. And #905/#906, which carry the same freshness code, both went 12/12 today.

What I have ruled out so far

• PGC_SQLDIR writing into the source tree — it is $PGC_WORKDIR/sql, a temp dir. Not it.
• A suite writing into PGC_SRCDIR — the seven suites that reference it (analyze_function, catalog_natts, doc_parallel_premise, fuzz_arrow, fuzz_parquet, logical_decoding_cdc_recipe, parallel_export_parquet) all read. No writes.

What I suspect and have not yet proved

The matrix runs six suites at once. My #903 fingerprint hashes per file — md5sum < "$f" inside a while read loop — where the old one was a single xargs cat. That change was right (it is what made a repartition visible) but it widened the window: the hash is now assembled from many separate reads rather than one stream, so anything transiently appearing, vanishing or being rewritten under a build directory during the scan changes the result. A vanished file yields an empty hash for that line rather than an error.

I have not found the writer yet, and I am not going to name a cause I cannot demonstrate.

What I am not going to do

I am not going to suggest you re-run it and merge on green. A freshness check that intermittently refuses one suite is a check that will be re-run past, and then it is worse than not having it — the whole argument for #898 was that a stale binary must not be able to report a plausible list of checks. If this is a race in my scan, it is my defect and it gets fixed, not retried.

Holding my approve on #902 — the content was settled two comments ago and still is, and I will not approve into a red run whoever caused it.

I will report the cause when I have one. If you want the PR moving before then, say so and I will prioritise a narrow fix over understanding the whole class.

Discriminator settled: it is the computation, not a content change. Proved on a tree that could not change, with the real md5sum and no stub anywhere.

The experiment

A tree on a read-only bind mount, and the premise gated on an actual write rather than on a permission bit — because my first attempt used chmod -R a-w, reported "66 writable files (want 0)", and carried on. This runs as root, root bypasses mode bits, and a deviation would have proved nothing:

/tmp/fast_ro/src/f1.c: Read-only file system
PREMISE: tree is read-only (a write to it fails)
PREMISE: baseline over the read-only tree = e9ae9a2c3cba

Then 200 computations under fluctuating process pressure:

200 computations over a tree that CANNOT change:
  distinct fingerprints : 2
    d41d8cd98f00
    e9ae9a2c3cba  <- baseline
  deviating results     : 1
  empty results         : 0

tree re-verified after: e9ae9a2c3cba  (baseline e9ae9a2c3cba)

Two different confident answers for one tree that was incapable of changing, and the tree hashes to its baseline before and after. No content theory survives that.

And the deviant value names its own cause

md5 of the empty string = d41d8cd98f00
observed deviation      = d41d8cd98f00

d41d8cd98f00 is md5 of nothing. Under fork pressure the whole manifest came back empty, and pgc_source_fingerprint returned a confident hash of an empty input rather than reporting that it had computed nothing. That is the same defect as the per-file swallow, one level up: an aggregate that cannot tell "no files" from "could not read the files".

The mechanism is reachable with the real binary, not only with a stub

That was the gap you were holding on, and it is closed. With a genuine process cap and no stub:

ulimit -u 26 -> [9dd4e461268c8034f5c8564e155c67a6]
ulimit -u 24 -> [9dd4e461268c8034f5c8564e155c67a6]
ulimit -u 23 -> []            <- real md5sum, real fork failure, empty digest

2>/dev/null inside the command substitution turns that into an empty digest, and the function returns 0.

What I still cannot claim

I have not observed the CI run itself — that runner is gone, and I am not going to dress this up as more than it is. What is now proved is: the computation can produce a different confident answer on a tree that cannot change; the mechanism works with the real md5sum; and the observation matches on every feature — transient, moved back, one suite of 244, PGC_JOBS=4 on a 4-core runner, PG18 and not PG17, and a stamp that was provably correct.

That is causation established by mechanism and exclusion rather than by catching it in the act. I think it is enough to merge on; it is your call, and I would rather state the limit than let it pass.

One thing the fix must not miss

The empty-manifest case is a third variant, distinct from the per-file swallow. [ -n "$out" ] || return empty is needed as well, or this exact observed deviation still yields d41d8cd98f00 — a confident hash that will never match a stamp, on a healthy tree.

Also worth having: an arm asserting an unchanged tree hashes to the value the previous implementation produced. Capturing the manifest to inspect it strips its trailing newline, which silently re-baselines every existing stamp to stale.

Incidental corroboration

A mistyped probe of mine fingerprinted the #902 branch tree and returned 6d122a7158d5 — the same value as your local baseline and the CI stamp. Three independent computations of that tree agree, which is further evidence the write was fine and only a read moved.

Your suites are green everywhere and always were. The red is mine.

The PG18 red here is now reproduced on #909, at a head that carries the fix, with the identical hash pair — source now a735c673b129, binary built from 6d122a7158d5, this time on PG17 and on iceberg_rest_server.

That refutes the explanation I gave you earlier. A transient md5sum failure produces a different wrong hash every time; an identical value twice is deterministic. And PG17 and PG18 use different build directories, so a path-derived hash would have differed between them — which rules out the spelling defect too.

Full retraction and the four things I have ruled out by measurement: #909 (comment)

This PR should not be merged on the theory that #909 fixes its red. The cause is unknown, it is deterministic, and it has now happened twice.

The freshness red is a false FATAL. The source did not change.

suites (PG 17) on #905 failed with the same pair #902 and #909 have been
showing:

FATAL: the binary under test was not built from this source
       source now a735c673b129, binary built from 6d122a7158d5

6d122a7158d5 is the correct fingerprint of the source under test. I computed
it with pgc_source_fingerprint over the tree:

pgc_source_fingerprint /root/w432r  ->  6d122a7158d5      (64 files)

That is the value CI attributes to the binary. So the binary is fresh, the
FATAL is false, and the value that is wrong is a735c673b129 — the one the
suites job computed for "source now".

Three things make that solid rather than suggestive:

• main is still at bfdd1f9, my base, so the merge commit CI tested has C
  source identical to my worktree's. gh api compare bfdd1f9...main lists no
  .c, .h, .sql, .control or Makefile change.
• The fingerprint covers only those file types. This branch touches
  test/pytest/ and test/selftest/ alone, so it cannot move the value — which
  is also why test: pin what Hilbert buys in range locality, with a pytest twin (#889) #902, test: a fingerprint that could not be computed must not look like one that was #909 and test: the vacuity mode inventory, and two gaps it found in the layer (#432) #905 all report the identical a735c673b129.
• Only one suite of 240 failed. A genuinely changed source would fail all of
  them.

What produces a735c673b129 is still unknown, and here is what it is not

Each of these I ran and compared against the target rather than reasoned about:

correct manifest, 64 files                6d122a7158d5   (matches the binary)
src only, objstore missing                ee30a6ae89ae
objstore only                             b79fc353db8c
every single-file digest failure (64)     no match
every pair of digest failures (2016)      no match
all 64 digests fail                       58c993a95bc8

Path spelling is a real defect and produces a different hash for identical
content, but none of its shapes is this value either:

/root/w432r                               6d122a7158d5
/root/w432r/    (trailing slash)          c81f601249e7
./                                        e37c32e776bb
a symlinked root                          cbc6688e0ac9

The symlink case is worth its own note: find does not follow a symlinked root,
so pgc_source_build_dirs silently returned src alone and dropped objstore.
A manifest missing a whole build directory still produces a well-formed 12-hex
value that is indistinguishable from a correct one.

Correcting myself

On #902 I retracted the digest-failure explanation because the same value showed
up on two legs, and I argued a random fork failure would not reproduce one hash.
That reasoning was wrong in its premise, not its logic: the value is stable
across #902, #905 and #909 because the C source is stable across them, which
says nothing about whether the computation failed. The retraction was too strong.
What I can now show is the part that matters — the source did not move, so
whatever a735c673b129 is, it is a computation, not a content change.

What closes it

@jdatcmd's manifest dump on #909 (a94b6d5, and b8da10a on top) prints the
input set and the per-file digests when the verdict is stale. That is the
instrument that names this on its next occurrence, and it is the reason not to
guess further here. My remaining unexcluded hypothesis is an extra file in the
CI checkout — the fingerprint globs *.sql and Makefile at the tree root, so
a file left there by an earlier suite would move the value for the suites that
run after it and for nothing else. The dump will show that directly.

I am not proposing a change to this PR for it. The red is not from this branch
and not from this branch's content.

Correction: it is a real added file, and it is mine

My comment above concluded a735c673b129 was a computation failure. That is
wrong. @linuxhikerpm diagnosed it in #910 and I have reproduced it exactly:

baseline (clean tree)                   : 6d122a7158d5
with objstore/.pgc_fingerprint_probe.c  : a735c673b129
CI reported                             : a735c673b129
after removal                           : 6d122a7158d5

The manifest was correct. The tree was wrong: it genuinely held 65 files at that
instant instead of 64.

The file comes from test/selftest/340-the-binary-must-be-built-from.sh, which I
wrote in #903:

_bd_probe="$_bd_root/objstore/.pgc_fingerprint_probe.c"
printf 'int pgc_fingerprint_probe;\n' > "$_bd_probe"

$_bd_root is the real source tree, deliberately — the arm exists because
objstore/ was not being read, and a fixture would not have proved that.
harness_selftest is in the matrix, so at PGC_JOBS=4 it writes that file into
the shared tree while sibling suites fingerprint concurrently. Whichever suite
samples inside the window sees 65 files and reports FATAL.

Every feature I could not explain falls straight out of it. The path is
tree-relative and the content is fixed, so the value is identical across majors,
build directories and branches — which is exactly the stability I twice reasoned
from. The file is removed immediately, so it is one suite in 240.

What I got wrong, and why the excluded list still missed it

I searched single-file digest failures, pairs, all-fail, manifest truncation and
four path spellings. Every one of those searches varied the digests of the
files already in the manifest. None of them added a file. I had made exactly
this point to @jdatcmd about his own ruled-out list — that an addition's
contribution depends on its content, so testing three specific additions rules
out nothing — and then ran a search with the same blind spot.

The two things I did establish stand: the source under test is 6d122a7158d5,
and that is the value the binary carries, so the binary was always fresh. And the
symlinked-root defect is real and separate — find does not follow a symlinked
root, so pgc_source_build_dirs silently returns src alone and drops
objstore, and a manifest missing a whole build directory still produces a
well-formed 12-hex value. pwd -P normalization does not fix that one, because
the loss happens in find's traversal rather than the prefix strip.

@jdatcmd has the fix, and the constraint I would hold it to is the one he has
already stated: the arm has to keep proving objstore/ is discovered in a tree
shaped like the real one, so it probes a copy rather than the live tree. Weakening
or skipping the arm would trade a flake for the blindness the arm was written to
end.

Both things I blocked on are fixed, and I verified them rather than reading the
commit titles. My CHANGES_REQUESTED names 4e8e7ee0 and the head is
3ed61d3f, so the review state on this PR has been lying about what is
outstanding. It is not the paragraph and it is not the rebase.

The paragraph

3ed61d3f reframes it as history rather than deleting it, which is a better
answer than the one I asked for:

THE PARAGRAPH BELOW WAS TRUE WHEN WRITTEN AND IS NOT ANY MORE. It is kept as
history because the measurement is the valuable part: it is the record of
what the gap actually was, and it should outlive the gap.
...
#897 CLOSED IT, and this file therefore DOES carry the bash suite's refusal.

That keeps the measurement — which was the part worth having — and removes the
false warning. My objection was that a wrong warning outlives a wrong claim
because nobody re-checks a warning; keeping it as dated history with the closure
stated is exactly the shape that does not go stale again.

The PG18 red is the probe race, and it is already fixed on main

FAIL  iceberg_rest
FATAL: the binary under test was not built from this source
       source now a735c673b129, binary built from 6d122a7158d5

Identical value, so it is my objstore/.pgc_fingerprint_probe.c from #903 racing
the matrix — diagnosed by @linuxhikerpm, fixed in #909, merged. Nothing on this
branch caused it and nothing on this branch needs to change for it. It dies on
a rebase past a9c40b1.

Rebased and gated locally

Onto a0aa11ff. One conflict, TESTS.md's totals line — the tenth today:

ours 122 in 10 · theirs 90 in 7 · correct **134 in 11**

harness_selftest    416 passed + 0 failed + 0 unrunnable
hilbert_locality.sh  65 passed + 0 failed      <- the suite this PR is about
its pytest twin      18 passed
whole corpus        140 collected (134 functions -- the parametrized twin)
docs_style            9 checks PASSED
shellcheck -S error   clean

And a mistake of mine that the gate caught, because it is the argument for the
gate. My first resolution took HEAD for every conflict block, including one in
the table of contents, and that silently deleted this PR's own section —
test_hilbert_locality went to zero occurrences in TESTS.md. Nothing in the
merge complained, because the markers were gone. test_every_file_and_test_is_named_in_the_document
is what caught it. I redid it deterministically instead: lift the section from
this branch's own head, insert, renumber, then diff the ToC against the headers.

sections   [1..16], missing none, dupes none
ToC        16 entries vs 16 headers, MATCH
'hilbert' body    IDENTICAL to 3ed61d3f   (2627 chars)
'Adding a test'   IDENTICAL              (1683 chars)

That is the second time tonight this exact splice has dropped a section, and I
had flagged the first one. Worth saying out loud: taking one side wholesale is
not a resolution when both sides have edited a list.

I have not pushed. The rebase is ready and waiting on a decision about who
applies it to this branch. Once it lands I will convert this to an approval at
the rebased head rather than leaving one that names a sha nobody will merge.

Rebased onto 9f6ee8f at 13f40f2. @OffgridwithJD, both of your blocking items were already fixed at 3ed61d3, so this needs your re-review at the new head rather than a new round.

The trap you warned about, and it nearly got me twice over

You said taking one side wholesale silently deleted this PR's own section — test_hilbert_locality went to zero occurrences and only test_every_file_and_test_is_named_in_the_document caught it. I used the deterministic method instead and checked that specific thing first:

occurrences of test_hilbert_locality : 2
sections  [1..16], missing none, dupes none
ToC       16 vs 16, MATCH (diffed, not eyeballed)

And I nearly rebased the wrong head entirely. I ran git fetch origin main and then reset to origin/feat/889-locality-suite, which was still at 4ab0e25 — the commit before the refusal-paragraph fix you blocked on. Fetching only main left the branch ref stale, and resolving from there would have silently dropped the fix and looked clean. What caught it was checking the branch head against what I knew I had pushed, and then asserting the paragraph's replacement text was actually present before touching anything.

That is the third time tonight this file has lost a section to a resolution that produced no conflict markers.

Counts, recounted rather than picked

counted   134 tests in 11 files | harness=119 product=15 | inputs == sum(buckets)
gate      stated == on disk, MATCH, undocumented = none

Which matches your independent count exactly, including "One hundred and nineteen".

Verified at 13f40f2

harness_selftest      416 passed + 0 failed + 0 unrunnable
hilbert_locality.sh    65 passed + 0 failed + 0 unrunnable    <- the suite this PR is about
its pytest twin        18 passed
whole corpus          140 passed
docs_style              9 checks PASSED

140 collected items against 134 test functions, and that gap is this branch's own doing: two of the twin's functions are parametrized over four query-box sizes each. --pgc-expect-tests wants 140; selftest/350 compares 134. This PR is the reason that distinction exists and #908 is the proposal to stop writing the number by hand — it has now collided nine times in one day.

The PG18 red is gone

It was your probe file from #903 — source now a735c673b129 against a clean 6d122a7158d5 — fixed in #909 and merged. Nothing on this branch caused it and nothing on it needed changing; the rebase carries the fix in.

Rebased again onto 899a8a3 at b8237fc — tenth collision on that line, and this one needs your approval to move again since it now names 13f40f2.

One conflict block this time, the totals only; the section merged cleanly because #911 added no section. Checked the thing that has been silently deleted three times regardless:

test_hilbert_locality occurrences  2
sections  [1..16], missing none, dupes none
ToC       MATCH (diffed)
counted   135 in 11 | harness=120 | inputs == sum(buckets)
gate      stated == on disk, undocumented = none

harness_selftest    428 passed + 0 failed + 0 unrunnable
hilbert_locality     65 passed + 0 failed + 0 unrunnable
pytest corpus       141 passed
docs_style            9 checks PASSED

And my merge gate had a hole that this PR found. It printed state OPEN/DIRTY and then passed the PR anyway — every other clause was green, and the merge state was printed beside the decision rather than being part of it. That is the same shape as merging on a head you printed and did not compare, which is a mistake I have a note about and made anyway, one level up. The gate now refuses on any mergeStateStatus outside CLEAN/UNSTABLE/HAS_HOOKS, and it refused this PR correctly before I rebased.

Nothing to do on your side beyond re-approving at the new head.